Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BIP374: Discrete Log Equality Proofs (DLEQ) #1689

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

andrewtoth
Copy link
Contributor

@andrewtoth andrewtoth commented Oct 24, 2024

This BIP specifies a standard way to generate and verify DLEQ proofs. This is motivated by sending to silent payments in PSBTs. However, there are also other uses where DLEQs could be useful, so it would be good to have this BIP for others to reference.

This is inspired by https://github.com/discreetlogcontracts/dlcspecs/blob/master/ECDSA-adaptor.md#proof-of-discrete-logarithm-equality, but is a little more specific.
There is an implementation of that already at https://github.com/BlockstreamResearch/secp256k1-zkp/blob/master/src/modules/ecdsa_adaptor/dleq_impl.h, which this BIP attempts to be compatible with.

Inital ML post: https://groups.google.com/g/bitcoindev/c/MezoKV5md7s

@andrewtoth
Copy link
Contributor Author

There was some previous discussion on this gist before making this PR
https://gist.github.com/andrewtoth/df97c3260cc8d12f09d3855ee61322ea

bip-DLEQ.mediawiki Outdated Show resolved Hide resolved
bip-DLEQ.mediawiki Outdated Show resolved Hide resolved
@jonatack jonatack changed the title Bip Draft: DLEQ Bip Draft: Discrete Log Equality Proofs (DLEQ) Oct 24, 2024
bip-DLEQ.mediawiki Outdated Show resolved Hide resolved
bip-DLEQ.mediawiki Outdated Show resolved Hide resolved
@jonatack jonatack added the PR Author action required Needs updates, has unaddressed review comments, or is otherwise waiting for PR author label Oct 26, 2024
bip-DLEQ.mediawiki Outdated Show resolved Hide resolved
bip-DLEQ.mediawiki Outdated Show resolved Hide resolved
bip-DLEQ.mediawiki Outdated Show resolved Hide resolved
bip-DLEQ.mediawiki Outdated Show resolved Hide resolved
@andrewtoth
Copy link
Contributor Author

Thanks for your comments @jonatack, @stratospher, @theStack. I've also updated the BIP to include the generator G as an input, and so the BIP is no longer specific to secp256k1. This was mentioned on the mailing list as an improvement to make this standard work with other curves as well.

@jonatack jonatack removed the PR Author action required Needs updates, has unaddressed review comments, or is otherwise waiting for PR author label Nov 5, 2024
Copy link
Contributor

@murchandamus murchandamus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From an editorial standpoint this looks good so far. As mentioned by Jon, please include a Backwards Compatibility section, if only to state that there are no concerns.

I did not verify the cryptography of the proof, but after staring at it for a few minutes, I and perhaps other would perhaps benefit from a couple sentences of why/how the proof works e.g. as a footnote. I was also wondering whether you might want to expand on related work, alternate designs, and design decisions in this document. For example you might want to mention some of the things from the opening comment on the PR here, in the footnotes.

@theStack
Copy link
Contributor

Fwiw, I've written a reference implementation of this BIP for secp256k1 in Python, see: https://github.com/theStack/bips/blob/bip-DLEQ-add_reference_impl/bip-DLEQ/reference.py
It's probably useful to be added to the BIP and also to create test vectors. The unit tests can be executed via

$ cd bip-DLEQ
$ python3 -m unittest reference.py

* Fail if ''k = 0''.
* Let ''R<sub>1</sub> = k⋅G''.
* Let ''R<sub>2</sub> = k⋅B''.
* Let ''e = int(hash<sub>BIP0???/challenge</sub>(cbytes(A) || cbytes(B) || cbytes(C) || cbytes(G) || cbytes(R<sub>1</sub>) || cbytes(R<sub>2</sub>)))''.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I wonder if it is really needed to also include the generator point in the challenge hash? Seems excessive to me as its implicitly included in all other points. Generally I'm not sure what are the best practices here, since this seems to be the first BIP where the generator point can be generic and is not defined as the one in secp256k1.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure we can remove this? I added it to reference.py in case.

@murchandamus murchandamus added the PR Author action required Needs updates, has unaddressed review comments, or is otherwise waiting for PR author label Nov 25, 2024
bip-DLEQ.mediawiki Outdated Show resolved Hide resolved
bip-DLEQ.mediawiki Outdated Show resolved Hide resolved
andrewtoth and others added 3 commits December 10, 2024 19:17
Co-authored-by: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
Co-authored-by: Sebastian Falbesoner <sebastian.falbesoner@gmail.com>
@jonatack jonatack removed the PR Author action required Needs updates, has unaddressed review comments, or is otherwise waiting for PR author label Dec 11, 2024
Squashed from the following commits:
- Add skeleton for generating DLEQ proof test vectors
- Add run_test_vectors.py counterpart for generated DLEQ proofs
- Add DLEQ test vectors for proof verification
@theStack
Copy link
Contributor

A first draft for DLEQ test vectors is available now (special thanks to @stratospher for discussing the BIP and collecting test ideas together!):

https://github.com/theStack/bips/tree/bip-DLEQ-add_test_vectors (commit theStack@6b16952)

They can be generated and ran by:

$ cd bip-DLEQ
$ ./gen_test_vectors.py
$ ./run_test_vectors.py

There are probably more failure and edge cases possible, but they at least already paid off by revealing a bug in the reference implementation (see comments below; the fix is included as a separate commit in the branch). Also, by now only secp256k1's generator point is exercised. Further test ideas and comments are very welcome!

@jonatack @murchandamus: Is there still something to be done for assigning a BIP number? 🤠

bip-DLEQ/reference.py Outdated Show resolved Hide resolved
bip-DLEQ/reference.py Outdated Show resolved Hide resolved
bip-DLEQ.mediawiki Outdated Show resolved Hide resolved
@andrewtoth
Copy link
Contributor Author

I did not verify the cryptography of the proof, but after staring at it for a few minutes, I and perhaps other would perhaps benefit from a couple sentences of why/how the proof works e.g. as a footnote. I was also wondering whether you might want to expand on related work, alternate designs, and design decisions in this document. For example you might want to mention some of the things from the opening comment on the PR here, in the footnotes.

@murchandamus I added some text on a few design decisions, as well as a description section describing the algebra of the proof.

@murchandamus
Copy link
Contributor

Thanks!

@jonatack @murchandamus: Is there still something to be done for assigning a BIP number? 🤠

I was waiting for the review to be addressed, but now it’s back on my review list. I’ll consider that point when I get around to review.

Copy link
Contributor

@murchandamus murchandamus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks pretty advanced for a first draft. Thanks! I did notice that there was only a little rationale, and wanted to ask whether there were any more notable questions or alternate approaches that should perhaps be mentioned. Other than that, this look great already.

Let’s call this BIP 374. Could you please add the entry to the table in the README?


=== Abstract ===

This document proposes a standard for 64-byte zero-knowledge ''discrete logarithm equality proofs'' (DLEQ proofs) over an elliptic curve. For given elliptic curve points ''A'', ''B'', ''C'', and ''G'', the prover proves knowledge of a scalar ''a'' such that ''A = a⋅G'' and ''C = a⋅B'' without revealing anything about ''a''. This can, for instance, be useful in ECDH: if ''A'' and ''B'' are ECDH public keys, and ''C'' is their ECDH shared secret computed as ''C = a⋅B'', the proof establishes that the same secret key ''a'' is used for generating both ''A'' and ''C'' without revealing ''a''.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is obvious to other readers, but there doesn’t seem to be any information regarding how ''B'' is picked in this process. Is it provided per the context of the challenge or picked by the prover? Would there e.g., be an issue if the prover knew ''b''?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

''B'' can be any point on the curve. Whether ''b'' is known to the prover or verifier does not change the generation or verification. If the prover substitutes ''b'' for ''a'' it would invalidate the proof. This is true for ''G'' as well.

I modified the second sentence slightly to clarify. Let me know if you think it could be worded more clearly.

bip-DLEQ.mediawiki Outdated Show resolved Hide resolved
bip-DLEQ.mediawiki Outdated Show resolved Hide resolved
@murchandamus murchandamus changed the title Bip Draft: Discrete Log Equality Proofs (DLEQ) BIP374: Discrete Log Equality Proofs (DLEQ) Dec 26, 2024
@murchandamus
Copy link
Contributor

There was some previous discussion on this gist before making this PR https://gist.github.com/andrewtoth/df97c3260cc8d12f09d3855ee61322ea

Oh, and could you add that gist to the Post-History header, if you think it would be relevant for future readers, or alternatively, document any relevant bits here?

@andrewtoth
Copy link
Contributor Author

Oh, and could you add that gist to the Post-History header, if you think it would be relevant for future readers, or alternatively, document any relevant bits here?

I added that and the mailing list link 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants